Skip to content

Refactor MSL authentication checks#606

Open
MattyTheHacker wants to merge 92 commits into
mainfrom
msl-auth
Open

Refactor MSL authentication checks#606
MattyTheHacker wants to merge 92 commits into
mainfrom
msl-auth

Conversation

@MattyTheHacker
Copy link
Copy Markdown
Member

depends on #605

@MattyTheHacker MattyTheHacker self-assigned this Sep 1, 2025
@MattyTheHacker MattyTheHacker added refactor Improvements to the codebase that do not directly affect users sync Request bots to automatically keep this PR up to date with it's base branch labels Sep 1, 2025
@MattyTheHacker MattyTheHacker enabled auto-merge (squash) September 1, 2025 12:39
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@MattyTheHacker MattyTheHacker added the suspended Delayed until a later date label Sep 3, 2025
@automatic-pr-updater
Copy link
Copy Markdown
Contributor

This pull request has a merge conflict with the base branch! Please resolve the conflict manually, remove the conflict label and re-add the filter label (if applicable).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

utils/msl/core.py:52

  • self.cookies is typed/used as a mapping (passed to aiohttp.ClientSession(..., cookies=...) and indexed with [".AspNet.SharedCookie"]), but it’s initialised from settings["SU_PLATFORM_ACCESS_COOKIE"], which is configured as a raw string in config.py. This will break request construction and cookie refresh logic; initialise self.cookies as {'.AspNet.SharedCookie': settings['SU_PLATFORM_ACCESS_COOKIE']} (or change the settings value to be a mapping consistently).
        self.headers: Mapping[str, str] = settings["SU_PLATFORM_WEB_HEADERS"]
        self.cookies: Mapping[str, str] = settings["SU_PLATFORM_ACCESS_COOKIE"]

    async def fetch_url_content(self, url: str) -> str:
        async with (
            aiohttp.ClientSession(headers=self.headers, cookies=self.cookies) as http_session,
            http_session.get(url=url, ssl=GLOBAL_SSL_CONTEXT) as http_response,
        ):
            returned_asp_cookie: Morsel[str] | None = http_response.cookies.get(
                ".AspNet.SharedCookie"
            )
            if not returned_asp_cookie:
                return await http_response.text()

            if returned_asp_cookie.value != self.cookies[".AspNet.SharedCookie"]:
                logger.info("New SU platform access cookie given by server; updating local.")
                self.cookies = {
                    ".AspNet.SharedCookie": returned_asp_cookie.value,
                }

cogs/check_su_platform_authorisation.py:80

  • The f-string used to format the organisation list is syntactically invalid (f"\n{ followed by a newline). This will raise a SyntaxError and prevent the cog from importing; build the joined string in a variable (or keep the {...} expression on a single line) before interpolating it.
            await ctx.followup.send(
                content=(
                    "No MSL organisations are available to the SU platform access cookie. "
                    "Please check the logs for errors."
                    if not su_platform_access_cookie_organisations
                    else (
                        f"SU Platform Access Cookie has access to the following "
                        "MSL Organisations:"
                        f"\n{
                            ',\n'.join(
                                organisation
                                for organisation in su_platform_access_cookie_organisations
                            )
                        }"
                    )

Comment thread utils/msl/core.py Outdated
Comment thread utils/msl/authorisation.py
Comment thread utils/msl/__init__.py
Comment thread cogs/check_su_platform_authorisation.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread utils/msl/core.py Outdated
Comment thread utils/msl/authorisation.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improvements to the codebase that do not directly affect users sync Request bots to automatically keep this PR up to date with it's base branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants